Skip to content

headplot func w test runner#49

Open
ngup1 wants to merge 5 commits intomainfrom
feature/nilesh/head-plot
Open

headplot func w test runner#49
ngup1 wants to merge 5 commits intomainfrom
feature/nilesh/head-plot

Conversation

@ngup1
Copy link
Collaborator

@ngup1 ngup1 commented Dec 1, 2025

-added modern headplot function following finns structure
-added runner test file to test generating plots

@madhuritha22
Copy link
Collaborator

The head-plot module and standalone script look solid and useful for export workflows.
However — I don’t see integration code that adds head-plots to the GUI’s graph viewer flow yet. As is, head-plots seem only accessible via the script runner, not via the in-app visualization.
Could you clarify whether GUI integration is planned for this sprint? If yes, please add wiring in GraphViewerScene (or equivalent) to call plot_head() when appropriate.
If not — let’s document clearly that head-plots are only available through the export script.

@Omanhene20
Copy link
Collaborator

Module Architecture - Integration Concern

I see the headplot function follows Finn's structure which is good for consistency.
However, I agree with @madhuritha22's concern about GUI integration.

Questions:

  1. Is there a specific reason head plots are separated from the main GraphViewerScene workflow?
  2. What's the technical barrier to adding it to the GUI alongside fin/tail plots?

Suggestion:
Consider creating a PlotRegistry or PlotFactory pattern that allows easy registration
of new plot types. This would make adding future plot types (spine plots, movement overlays)
more maintainable:

class PlotRegistry:
    _plotters = {}
    
    @classmethod
    def register(cls, name, plotter_func):
        cls._plotters[name] = plotter_func
    
    @classmethod
    def get_available_plots(cls):
        return list(cls._plotters.keys())

@Omanhene20
Copy link
Collaborator

Testing Coverage** ⭐

On the test runner file, add:

**Automated Testing - Test Coverage**

Good initiative adding a test runner! However, I have some concerns about test comprehensiveness:

**Current State:**
- ✅ Tests plot generation (good!)
- ❌ Missing edge case tests
- ❌ No validation of plot output correctness

**Recommended Test Cases:**
1. **Empty data**: What happens when the CSV has no valid frames?
2. **NaN values**: How does the plot handle missing head coordinates?
3. **Single frame**: Edge case with only 1 data point
4. **Invalid config**: Missing required fields in config JSON
5. **Output validation**: Check that generated plots have expected data ranges

**Example test structure:**
```python
def test_headplot_empty_data():
    """Test head plot handles empty DataFrame gracefully."""
    empty_df = pd.DataFrame()
    config = load_test_config()
    
    # Should either skip gracefully or raise informative error
    with pytest.raises(ValueError, match="No data to plot"):
        plot_head(empty_df, config)

def test_headplot_output_has_correct_traces():
    """Verify generated plot contains expected data."""
    df = load_test_data()
    config = load_test_config()
    
    fig = plot_head(df, config)
    assert len(fig.data) > 0  # Has trace data
    assert fig.layout.xaxis.title.text == "Time (s)"  # Correct axis labels

Adding these tests would prevent regressions and improve code reliability

# Head Plot GUI Integration

## Overview
Head plots are now integrated into the GraphViewerScene and appear automatically in the GUI when calculations complete, following the same pattern as dot plots. Both use the DataFrame directly for optimal performance.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Is there a specific reason head plots are separated from the main GraphViewerScene workflow?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants